Declare com.microsoft.azure:adal4j dependency as optional #22584
Conversation
|
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 2bdf318
Failures⚙️ Native Tests - Data1 #- Failing: integration-tests/jpa-mssql
📦 integration-tests/jpa-mssql✖ |
|
@Sanne The native image generation fails with the following error, since I marked a dependency as optional. I don't really know much about native image generation using GraalVM. To me it seems like it would make sense to only native-compile this method if also the optional adal4j dependency is actually on the classpath. Is there some way to achieve this? |
You would likely need to make |
|
Thanks for the tip! I will look into it. |
|
Hmm... The substitute actually already exists: Apparently an attempt is still made to load the substituted class. All fields and methods in that class have been substituted. The mentioned class |
Yes my point was that it might need to be conditionally substituted. |
knutwannheden
force-pushed
the
make-adal4j-optional
branch
2 times, most recently
from
9488690 to
6a13582
Compare
|
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 6a13582
Failures⚙️ Initial JDK 11 Build #- Failing: extensions/jdbc/jdbc-mssql/runtime
! Skipped: devtools/bom-descriptor-json docs extensions/jdbc/jdbc-mssql/deployment and 2 more📦 extensions/jdbc/jdbc-mssql/runtime✖ |
The `quarkus-jdbc-mssql` extension has a dependency on `com.microsoft.azure:adal4j`, which apparently is required for compilation to succeed. To allow Quarkus applications to work with newer versions of `com.microsoft.sqlserver:mssql-jdbc` (currently not possible in Quarkus, because newer versions have issues with native mode), this dependency is declared as optional, since the newer versions have replaced this dependency with `com.azure:azure-identity` to support Active AD based authentication. Also note that this dependency is also declared as optional by the `mssql-jdbc` module and that a Quarkus application unfortunately cannot work with Maven exclusions to exclude the dependency when imported via the Quarkus BOM.
knutwannheden
force-pushed
the
make-adal4j-optional
branch
from
6a13582 to
6ee3e24
Compare
Failing Jobs - Building 6ee3e24
Failures⚙️ Native Tests - Data1 #- Failing: integration-tests/jpa-mssql
📦 integration-tests/jpa-mssql✖ |
|
Maybe we should try upgrading the driver? I remember giving it a shot and then aborting as it was more complex than expected and couldn't dedicate some proper cycles to it, but we should look at that again. |
|
I cannot find a way to mark the adal4j dependency as optional and I probably tried some of the same things you did. I also stumbled across the oracle/graal#819 issue you created at some point. I am a bit surprised by this limitation. Trying to upgrade the driver would indeed be nice. That is also why I wanted to get rid of this dependency. I will give it another try and see how far I get. |
|
many thanks for giving it another shot @knutwannheden , appreciate it :) I've also pinged @galderz in hope we can restart a conversation with the graalvm about such issues. |
|
Basically I run into the same issues with the new driver. I could try to declare some of the driver's optional dependencies as non-optional (like you did for adal4j) and see if I get it to work that way. I suppose it would be possible to set compiler flags like |
Right that's too slippery: it's a global flag and introduces various other issues, which might even break other extensions as we've observed it having side-effects on other compiler flags. And I don't want the presence/absence of one extension to affect any other extension: the amount of permutations would make it all untestable. See also:
+1 to keep the dependencies around: dead code elimitation is going to get rid of the code anyway so it's not like it would produce a suboptimal native binary; at worst it will eat a little disk space in JVM mode. |
|
@knutwannheden so what should we do with this PR? Should we close it or do you plan to upgrade the driver here? Thanks! |
|
@knutwannheden friendly ping on #22584 (comment) here :) |
|
I tried everything I could think of but I failed. I lack the experience with GraalVM, but it seems like the tooling to do these kinds of things is also lacking a bit. |
|
@knutwannheden Indeed the tooling for getting around this issues is not great. This is why I created oracle/graal#3929. A couple of months ago I explored the feasibility of implementing it but couldn't find an easy way and had to move to other priorities. In the past few days a PR has been submitted that might find a way around your issues, but we're not fully satisfied with the way it works (see here). |
The
quarkus-jdbc-mssqlextension has a dependency oncom.microsoft.azure:adal4j, which apparently is required for compilation to succeed.To allow Quarkus applications to work with newer versions of
com.microsoft.sqlserver:mssql-jdbc(currently not possible in Quarkus, because newer versions have issues with native mode), this dependency is declared as optional, since the newer versions have replaced this dependency withcom.azure:azure-identityto support Active AD based authentication.Also note that this dependency is also declared as optional by the
mssql-jdbcmodule and that a Quarkus application unfortunately cannot work with Maven exclusions to exclude the dependency when imported via the Quarkus BOM.